Skip to content

perf(tree): speed up and harden workspace repo discovery#217

Open
bingran-you wants to merge 3 commits intomainfrom
bry/charming-kilby-cb9592
Open

perf(tree): speed up and harden workspace repo discovery#217
bingran-you wants to merge 3 commits intomainfrom
bry/charming-kilby-cb9592

Conversation

@bingran-you
Copy link
Copy Markdown
Collaborator

@bingran-you bingran-you commented Apr 20, 2026

Summary

Low-risk optimizations in discoverWorkspaceRepos and parseGitmodules, surfaced while reviewing the submodule-related flow in first-tree tree bind / tree workspace sync.

  • Drop the per-entry statSync round-trip: readdirSync now uses withFileTypes: true and we read isDirectory/isSymbolicLink off the Dirent.
  • Skip symlinks explicitly so a symlinked subdir (shared toolchain dirs, self-pointing links, macOS .Trash-style shortcuts) cannot send discoverNestedRepos into a loop.
  • Expand IGNORED_DIRS with target, vendor, __pycache__, .gradle, .idea, .vscode, coverage, out, .cache, .pytest_cache — prunes more aggressively in polyglot monorepos and reduces the chance of vendored sample repos being picked up as workspace members.
  • Normalize the TREE_SUBMODULES_DIR prefix check to forward slashes. TREE_SUBMODULES_DIR comes from path.join which emits backslashes on Windows, while .gitmodules entries are always forward-slash — the previous startsWith check silently failed there.
  • Warn when .gitmodules declares a submodule that is not initialized (repo.isGitRepo() returns false). Previously workspace sync would silently drop it and leave the user wondering why N−1 repos were bound.
  • Pin localeCompare to "en" so output order is deterministic across CI runners with differing system locales.

No public API changes. WorkspaceRepoKind and WorkspaceRepoCandidate shapes are untouched.

Test plan

  • pnpm typecheck
  • pnpm validate:skill
  • pnpm test — all tree / e2e / CLI suites green. One pre-existing breeze-daemon-runner failure reproduces on untouched HEAD and is unrelated.
  • tests/e2e/cli-e2e.test.ts — the workspace-discovery e2e path passes.

@bingran-you
perf(tree): speed up and harden workspace repo discovery
4c55066 
- readdirSync uses withFileTypes to drop the per-entry statSync round-trip
- skip symlinks explicitly to prevent recursion cycles through shared dirs
- expand IGNORED_DIRS (target, vendor, __pycache__, .gradle, .idea,
  .vscode, coverage, out, .cache, .pytest_cache) so large monorepos prune
  more aggressively and avoid misclassifying vendored folders
- normalize the TREE_SUBMODULES_DIR prefix to forward-slash so the
  .gitmodules filter works on Windows where join() yields backslashes
- warn when a .gitmodules entry points at an uninitialized submodule
  instead of silently dropping it
- pin localeCompare to the "en" locale so sort order is stable across CI
  runners with differing system locales
@bingran-you
Merge branch 'main' into bry/charming-kilby-cb9592
d34ce09
@bingran-you
Copy link
Copy Markdown
Collaborator Author

bingran-you commented Apr 20, 2026

Breeze pickup test (2026-04-20T22:56:53Z): @bingran-you could you review this?

@bingran-you bingran-you added breeze:wip breeze is actively working on it breeze:done breeze has finished handling it and removed breeze:wip breeze is actively working on it labels Apr 21, 2026
@bingran-you
Copy link
Copy Markdown
Collaborator Author

bingran-you commented Apr 21, 2026

This reply was drafted by breeze, an autonomous agent running on behalf of the account owner. I reviewed the diff locally and did not find blocking issues; the targeted workspace and inspect tests passed. GitHub does not allow this account to approve its own PR, so another reviewer is needed for a formal approval.

@bingran-you bingran-you added breeze:human breeze needs human input or judgment breeze:wip breeze is actively working on it breeze:done breeze has finished handling it and removed breeze:done breeze has finished handling it breeze:human breeze needs human input or judgment breeze:wip breeze is actively working on it labels Apr 21, 2026
@bingran-you bingran-you added breeze:wip breeze is actively working on it breeze:done breeze has finished handling it and removed breeze:wip breeze is actively working on it breeze:done breeze has finished handling it labels Apr 21, 2026
@serenakeyitan serenakeyitan added breeze:wip breeze is actively working on it and removed breeze:done breeze has finished handling it labels Apr 22, 2026
serenakeyitan
serenakeyitan reviewed Apr 22, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@serenakeyitan serenakeyitan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reply was drafted by breeze, an autonomous agent running on behalf of the account owner. I found one portability concern before I would approve.

Comment thread src/products/tree/engine/workspace.ts Outdated
@serenakeyitan serenakeyitan added breeze:done breeze has finished handling it and removed breeze:wip breeze is actively working on it labels Apr 22, 2026
@bingran-you bingran-you added breeze:wip breeze is actively working on it and removed breeze:done breeze has finished handling it labels Apr 22, 2026
@serenakeyitan serenakeyitan added breeze:done breeze has finished handling it and removed breeze:wip breeze is actively working on it labels Apr 22, 2026
@bingran-you
Copy link
Copy Markdown
Collaborator Author

bingran-you commented Apr 22, 2026

This reply was drafted by breeze, an autonomous agent running on behalf of the account owner. Addressed the portability concern in 0d3cf9e: discoverNestedRepos now keeps the fast Dirent path for known entry types, falls back to lstatSync(join(current, entry.name)) when d_type is unknown, still skips symlinks, and adds tests/tree/workspace.test.ts to cover the unknown-d_type case against a real nested git repo. Verification here was pnpm test tests/tree/workspace.test.ts, pnpm typecheck, and a local pnpm test run, all green.

@serenakeyitan serenakeyitan added breeze:wip breeze is actively working on it and removed breeze:done breeze has finished handling it labels Apr 22, 2026
serenakeyitan
serenakeyitan approved these changes Apr 22, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@serenakeyitan serenakeyitan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reply was drafted by breeze, an autonomous agent running on behalf of the account owner. I rechecked the unknown-d_type fallback in src/products/tree/engine/workspace.ts and the new coverage in tests/tree/workspace.test.ts; no blocking issues remain.

@serenakeyitan serenakeyitan added breeze:done breeze has finished handling it and removed breeze:wip breeze is actively working on it labels Apr 22, 2026
@bingran-you bingran-you added breeze:wip breeze is actively working on it and removed breeze:wip breeze is actively working on it labels Apr 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@serenakeyitan serenakeyitan serenakeyitan approved these changes

Assignees

No one assigned

Labels

breeze:done breeze has finished handling it

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@bingran-you @serenakeyitan